Skip to content

Conversation

@Attyuttam
Copy link

Summary

Changes

Fixes #2113 - cold start only for on-demand invocation type else its not considered


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@dreamorosi dreamorosi requested a review from phipag November 26, 2025 10:51
@dreamorosi dreamorosi removed the request for review from phipag November 26, 2025 10:51
Copy link
Contributor

@dreamorosi dreamorosi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Attyuttam thank you for the PR.

Before we can start with the review, please make sure that the project still compiles after having made changes. Currently the CI is failing across the board.

If you have questions, please let us know.

@phipag
Copy link
Contributor

phipag commented Nov 26, 2025

@Attyuttam The CI fails due to broken unit tests. You can run locally mvn clean test to verify tests before updating the PR.

My suggestion is to update the failing unit test by setting the environment variable you are checking for. You can use @SetEnvironmentVariable for this. Here is an example of a test doing something similar:

https://github.com/Attyuttam/powertools-lambda-java/blob/090de1d6d2d34f87820d242bc3a81a28d7945291/powertools-common/src/test/java/software/amazon/lambda/powertools/common/internal/LambdaHandlerProcessorTest.java#L150-L159

@Attyuttam
Copy link
Author

Attyuttam commented Nov 27, 2025

Still working on this, there are a lot of test failures due to the code change, will update and drop a message here !
Apologies for the delay, new to the ecosystem.

@phipag phipag self-requested a review November 27, 2025 10:05
@phipag
Copy link
Contributor

phipag commented Nov 27, 2025

No worries, feel free to reach out if you have any questions. Your work on this is much appreciated 👍

@phipag
Copy link
Contributor

phipag commented Nov 27, 2025

@Attyuttam For the other tests that are failing, most likely the issue will be resolved if you configure in pom.xml the Maven Surefire configuration to set the initialization type env var to on-demand. In this case, the cold start behavior should be exactly the same as before and the unit tests should pass for the other modules that are failing.

@Attyuttam
Copy link
Author

Ohh, okay. I was manually setting using the @SetEnvironmentVariable annotation on every test, I was about to ask this here. Thanks for the suggestion.

@Attyuttam
Copy link
Author

the powertools-idempotency-dynamodb module was failing in mvn clean test. Although, when the tests were run independently, they passed.

I looked at the blog that was put in the comments but that has become invalid, so I updated that to the correct URL as well.

I tried incorporating the changes based on the latest blog by nickolas fisher, but it did not work. Can you please help me out here

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: Support correct cold start detection for non-on-demand invocation types

3 participants